Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Migrating data to datastore #548

Merged
merged 27 commits into from
Feb 18, 2019
Merged

Migrating data to datastore #548

merged 27 commits into from
Feb 18, 2019

Conversation

maxsam4
Copy link
Contributor

@maxsam4 maxsam4 commented Feb 12, 2019 •
edited
Loading

  • Optimized Datastore (From 23.5kb to 11.5kb with 4 extra functions)
  • Moved investor list to datastore
  • Moved canBuy logic from GTM to STO
  • Added Investor flags
  • Renamed whitelist data to kyc data and moved flags to separate a uint256
  • Added eth gas reporter (it reports gas usage in tests, run using npm run gas)

@coveralls
Copy link

coveralls commented Feb 13, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.929% when pulling 86d5bab on universal-accredited into 32663e2 on dev-3.0.0.

@maxsam4 maxsam4 changed the title [WIP] Migrating data to datastore Migrating data to datastore Feb 14, 2019
@F-OBrien
Copy link
Contributor

I had a read through this pull request and wanted to reference a feature request I had raised #532
related to canBuyFromSTO.
This pull request goes part of the way implementing my suggestion by removing the canbBuyFromSTO check from the GTM and including it in the STO contracts (and also moving the accredited status out of the STO contract) but I believe it is still the case that setting canBuy for an investor allows them to buy from all STO's and does not allow the possibility to apply different investor restrictions to different STO's.
I still think the ability to have concurrent offerings with certain investors only allowed to invest in one or the other may be a useful feature in a future pull request if not in this one.

@maxsam4
Copy link
Contributor Author

maxsam4 commented Feb 14, 2019

Thanks for your feedback @F-OBrien! Different restrictions on different simultaneous STOs is a planned feature.

@adamdossa adamdossa merged commit 28fdd92 into dev-3.0.0 Feb 18, 2019
@satyamakgec satyamakgec deleted the universal-accredited branch October 30, 2019 12:17
Ttokkiyo
Ttokkiyo reviewed May 31, 2021
View reviewed changes
contracts/datastore/DataStore.sol
_;
}

function _isAuthorized() internal view {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok lov u

Ttokkiyo
Ttokkiyo reviewed May 31, 2021
View reviewed changes
contracts/storage/USDTieredSTOStorage.sol
@@ -30,7 +30,7 @@ contract USDTieredSTOStorage {
uint256 mintedDiscountPoly;
}

mapping(address => uint256) nonAccreditedLimitUSDOverride;
mapping(address => uint256) public nonAccreditedLimitUSDOverride;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mapping address is wrong D:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean ?

akashvekariya
akashvekariya reviewed Sep 15, 2021
View reviewed changes
contracts/datastore/DataStoreStorage.sol
uint8 constant DATA_KEY = 6;
bytes32 public constant MANAGEDATA = "MANAGEDATA";
uint8 internal constant DATA_KEY = 6;
bytes32 internal constant MANAGEDATA = "MANAGEDATA";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then this two variables won't be accessible

This was referenced Mar 22, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ttokkiyo Ttokkiyo Ttokkiyo left review comments

@akashvekariya akashvekariya akashvekariya left review comments

@satyamakgec satyamakgec satyamakgec approved these changes

@adamdossa adamdossa adamdossa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@maxsam4 @coveralls @F-OBrien @adamdossa @satyamakgec @akashvekariya @Ttokkiyo